Implemented migration rollback. #1932
Conversation
|
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #1932 +/- ##
============================================
+ Coverage 88.28% 88.66% +0.37%
- Complexity 7951 7980 +29
============================================
Files 242 243 +1
Lines 24211 24313 +102
============================================
+ Hits 21375 21557 +182
+ Misses 2836 2756 -80
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| } catch (PDOException $e) { | ||
| $this->createMigrationTable($name); | ||
| $migrationTimestamps = []; | ||
| } | ||
| } | ||
|
|
||
| sort($migrationTimestamps); | ||
| usort($migrationTimestamps, function (array $a, array $b) { | ||
| if ($a[static::COL_EXECUTION_DATETIME] === $b[static::COL_EXECUTION_DATETIME]) { |
There was a problem hiding this comment.
First off, nice, didn't know about the spaceship operator.
But why is this necessary? Is it even possible that ordering by execution date gives different results than ordering by migration timestamp?
Also, please don't repeat the execution date comparison. For me, this would be fine:
return $a[static::COL_EXECUTION_DATETIME] <=> $b[static::COL_EXECUTION_DATETIME]
|| $a[static::COL_VERSION] <=> $b[static::COL_VERSION];There was a problem hiding this comment.
According to the investigation that was made by @valerio8787, Propel migrate down command may roll back migration in the wrong order. E.g. we have two releases of the application, which contain the following migrations:
release 1 (migration applied 01.05.2022):
migration_001.php
migration_008.php
release 2 (migration applied 01.06.2022):
migration_005.php (migration created during development earlier than migration_008.php from release 1 and has an earlier version)
After the deployment of all versions, Propel migration table will have the following state:
version
--
001
008
005
Now if we run propel migrate:down command (that rollback only one migration) it will roll back migration with version 008 first, as Propel orders executed migrations by version and then rolls back a migration with max version value.
Speaking about sorting, we cannot use OR since we have to return -1, 0, 1, but in this case, it will return 0, 1.
There was a problem hiding this comment.
Thank you for clearing that up, that makes sense.
Also, you are right, I got the logical operator behavior confused with JavaScript. This should work though:
return $a[static::COL_EXECUTION_DATETIME] <=> $b[static::COL_EXECUTION_DATETIME]
?: $a[static::COL_VERSION] <=> $b[static::COL_VERSION];Or just a local variable:
$comparedDatetime = $a[static::COL_EXECUTION_DATETIME] <=> $b[static::COL_EXECUTION_DATETIME];
return ($comparedDatetime !== 0) ? $comparedDatetime : $a[static::COL_VERSION] <=> $b[static::COL_VERSION];but it is probably fine the way it is.
| } catch (PDOException $e) { | ||
| $this->createMigrationTable($name); | ||
| $migrationTimestamps = []; | ||
| } | ||
| } | ||
|
|
||
| sort($migrationTimestamps); | ||
| usort($migrationTimestamps, function (array $a, array $b) { | ||
| if ($a[static::COL_EXECUTION_DATETIME] === $b[static::COL_EXECUTION_DATETIME]) { |
There was a problem hiding this comment.
Thank you for clearing that up, that makes sense.
Also, you are right, I got the logical operator behavior confused with JavaScript. This should work though:
return $a[static::COL_EXECUTION_DATETIME] <=> $b[static::COL_EXECUTION_DATETIME]
?: $a[static::COL_VERSION] <=> $b[static::COL_VERSION];Or just a local variable:
$comparedDatetime = $a[static::COL_EXECUTION_DATETIME] <=> $b[static::COL_EXECUTION_DATETIME];
return ($comparedDatetime !== 0) ? $comparedDatetime : $a[static::COL_VERSION] <=> $b[static::COL_VERSION];but it is probably fine the way it is.
|
The PHPStan issue seems unrelated. |
|
@yaroslav-spryker @mringler Can you maybe approve again? |
MigrationMigrateCommand::execute()with providing a new optionmigrate-to-versionwhich allows defining the version to migrate database.MigrationStatusCommand::execute()with providing a new optionlast-versionwhich allows receiving the last executed version of the migration.